Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tests] Added smoke tests for conv solvers (ConvAsmBwdWrW1x1 and more). Some fixes. #1906

Merged
merged 79 commits into from
Mar 8, 2023

Conversation

atamazov
Copy link
Contributor

@atamazov atamazov commented Dec 19, 2022

@junliume https://github.com/ROCmSoftwarePlatform/MIOpen/labels/testing https://github.com/ROCmSoftwarePlatform/MIOpen/labels/urgency_high

Proposed reviewers: @averinevg @xinpin -- mostly to share knowledge, plus @shurale-nkn

TODO

  • Add GFX110X_ENABLED where necessary.

This PR adds smoke tests for conv solvers:

  • ConvAsmBwdWrW1x1 (with tuning)
  • ConvAsmBwdWrW3x3 (with tuning)
  • ConvAsmImplicitGemmGTCDynamicFwdXdlopsNHWC (with tuning)
  • ConvAsmImplicitGemmGTCDynamicBwdXdlopsNHWC (with tuning)
  • ConvAsmImplicitGemmGTCDynamicWrwXdlopsNHWC (with tuning)
  • ConvAsmImplicitGemmGTCDynamicFwdDlopsNCHWC (with tuning)
  • ConvCkIgemmFwdV6r1DlopsNchw (with tuning)
  • ConvHipImplicitGemmV4R1Fwd (with tuning)
  • ConvHipImplicitGemmV4R4Fwd (with tuning)
  • ConvHipImplicitGemmV4R4WrW (with tuning)
  • ConvHipImplicitGemmV4R1WrW (with tuning)
  • ConvHipImplicitGemmBwdDataV4R1 (with tuning)
  • ConvHipImplicitGemmBwdDataV1R1 (with tuning)
  • ConvHipImplicitGemmBwdDataV4R1Xdlops (with tuning)
  • ConvHipImplicitGemmForwardV4R4Xdlops (with tuning)
  • ConvHipImplicitGemmForwardV4R4Xdlops_Padded_Gemm (with tuning)
  • ConvHipImplicitGemmBwdDataV1R1Xdlops (with tuning)
  • ConvHipImplicitGemmFwdXdlops (with tuning)
  • ConvHipImplicitGemmForwardV4R5Xdlops (with tuning)
  • ConvHipImplicitGemmWrwV4R4Xdlops (with tuning)
  • ConvHipImplicitGemmWrwV4R4Xdlops_Padded_Gemm (with tuning)
  • ConvMlirIgemmFwd/Bwd/WrW (with tuning). Removed test_conv_igemm_mlir_small.
  • ConvMlirIgemmFwd/Bwd/WrWXdlops(with tuning). Removed test_conv_igemm_mlir_xdlops_small.
  • ConvBinWinogradRxSf2x3 (with tuning)
  • ConvBinWinogradRxSf2x3g1
  • ConvBinWinogradRxSf3x2 (with tuning)

By products:

  • [CMake][Fix] Fix issue in set_var_to_condition, see [HIP][COMGR] Use COMGR by default for HIP backend #1253 (review)
  • [NFC] ConvHipImplicitGemmBwdDataV1R1:
  • [tests][NFC]
    • Rename custom tests: smoke_conv_solver_Conv* -> smoke_solver_Conv*
    • Added SKIP_UNLESS_COMPOSABLEKERNEL option for custom tests
    • Added support for the "--output_type" option of test_conv.
    • Removed unnecessary MIOPEN_TEST_FLOAT_ARG from test_conv commands where TEST_CONV_VERBOSE_F/B/W is already used.
    • Rename couple of solver tests for clarity.
    • [NFC] Rename cmake variable: TEST_ANY_ERROR -> TEST_TUNING

…h results in omission of all custom tests
…d() check from ConvAsm1x1UV2 is it does not support FP16
…t potential performance degradation after tests.
@atamazov atamazov marked this pull request as ready for review January 30, 2023 13:15
@junliume
Copy link
Collaborator

@junliume @johnny-keker Ready for CI testing & review/merge!

Started CI on the latest commit.

@junliume
Copy link
Collaborator

junliume commented Feb 5, 2023

@junliume @johnny-keker Ready for the next round of CI testing.

@atamazov this PR has some issues with smoke_solver_ConvHipImplicitGemmFwdXdlops on gfx908

log.txt

@atamazov
Copy link
Contributor Author

atamazov commented Feb 7, 2023

@junliume

@atamazov this PR has some issues with smoke_solver_ConvHipImplicitGemmFwdXdlops on gfx908

The reason is #1971

@junliume
Copy link
Collaborator

@junliume @johnny-keker Ready for the next round of CI testing.

CI started

# RESOLVED Conflicts:
#	src/solver/conv_hip_implicit_gemm_bwd_v1r1.cpp
@atamazov
Copy link
Contributor Author

atamazov commented Mar 3, 2023

@junliume @johnny-keker Wait... there is an issue in smoke_solver_ConvHipImplicitGemmV4R1

@junliume
Copy link
Collaborator

junliume commented Mar 3, 2023

@junliume @johnny-keker Wait... there is an issue in smoke_solver_ConvHipImplicitGemmV4R1

@atamazov We recently updated it via #2005

…o override WORKAROUND_iGemm_936 from Jenkinsfile.
@atamazov
Copy link
Contributor Author

atamazov commented Mar 3, 2023

@junliume Thanks, I see. This is different issue related to W/A for #936 in Jenkinsfile.

@junliume @johnny-keker Ready for the next round of CI testing.

@atamazov
Copy link
Contributor Author

atamazov commented Mar 7, 2023

@junliume https://github.com/ROCmSoftwarePlatform/MIOpen/labels/TESTING_CI_PASSED can we go ahead and merge?

@junliume junliume merged commit 6fde4a8 into ROCm:develop Mar 8, 2023
@atamazov
Copy link
Contributor Author

atamazov commented Mar 8, 2023

@junliume Thanks!

Comment on lines +2287 to +2291
add_custom_test(smoke_solver_ConvBinWinogradRxSf2x3g1 GFX900_DISABLED GFX103X_ENABLED HALF_ENABLED SKIP_XNACK_ON
COMMAND MIOPEN_DEBUG_CONVOLUTION_ATTRIB_FP16_ALT_IMPL=0
MIOPEN_FIND_MODE=normal MIOPEN_DEBUG_FIND_ONLY_SOLVER=ConvBinWinogradRxSf2x3g1 $<TARGET_FILE:test_conv2d>
--input 1 40 20 20 --weights 20 40 3 3 --pads_strides_dilations 1 1 1 1 1 1 ${MIOPEN_TEST_FLAGS_ARGS}
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicate. Will be removed in next PR.

alexandraBara pushed a commit that referenced this pull request Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants